[SPEC] feat: init adaptive spec params from config#27493
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the initialization and validation of speculative decoding parameters, particularly for adaptive speculative decoding. It moves adaptive parameter setup earlier in the process, introduces helper functions _init_adaptive_speculative_params and _auto_choose_speculative_params within speculative_hook.py, and removes redundant code from server_args.py and adaptive_spec_params.py. Feedback on the changes highlights a missing import of SimpleNamespace in the newly added unit test, which will cause a NameError when executed.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def test_adaptive_defaults_to_config_step_when_spec_params_omitted(self): | ||
| with tempfile.NamedTemporaryFile("w", suffix=".json") as f: |
There was a problem hiding this comment.
The test uses SimpleNamespace to mock the model configuration, but SimpleNamespace is not imported in this file. This will cause a NameError when the test is executed. Please import SimpleNamespace from types.
def test_adaptive_defaults_to_config_step_when_spec_params_omitted(self):
from types import SimpleNamespace
with tempfile.NamedTemporaryFile("w", suffix=".json") as f:
|
/rerun-test test_adaptive_speculative.py test_adaptive_spec_params.py |
|
Results for 🚀 🚀 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79a6e3600d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| args.speculative_adaptive = True | ||
| args.speculative_adaptive_config = f.name | ||
| args.device = "cuda" | ||
| args.get_model_config = lambda: SimpleNamespace( |
There was a problem hiding this comment.
This new test uses SimpleNamespace but the module never imports it, so when this test runs with the normal test dependencies installed it raises NameError while constructing args.get_model_config and never exercises handle_speculative_decoding. Add from types import SimpleNamespace (as done in other tests) so the adaptive-default coverage can pass.
Useful? React with 👍 / 👎.
|
/tag-and-rerun-ci |
alphabetc1
force-pushed
the
feat/adaptive-spec-param
branch
2 times, most recently
from
f326e2f to
79a6e36
Compare
|
/rerun-test test_adaptive_speculative.py test_adaptive_spec_params.py |
|
Results for 🚀 🚀 |
2 participants
Summary
Tests
CI States
Latest PR Test (Base): ❌ Run #27249919715
Latest PR Test (Extra): ✅ Run #27263072560